node: add supply tracking to archive node#3973
Conversation
HDauven
left a comment
HDauven
left a comment
There was a problem hiding this comment.
The GraphQL endpoint and wiring look good, but the supply calculation has a few issues that need fixing before merge:
- Fees inflate the total supply.
total_block_rewardsincludes redistributed transaction fees, not just emissions. - Emission constants are off, they don't match what's in
rusk/src/lib/node.rs. I would suggest reusingemission_amount()directly. - f64 is problematic -> leads to
REALin SQLite, which risks prceision loss -> useINTEGER/u64.
| /// The name of the archive SQLite database. | ||
| const SQLITEARCHIVE_DB_NAME: &str = "archive.sqlite3"; | ||
| /// The initial supply for DUSK. | ||
| const INITIAL_SUPPLY: f64 = 500_000_000.0; |
There was a problem hiding this comment.
Any reason why you use f64/REAL over u64/INTEGER? INTEGER in SQLite is safer and would avoid floating point precision loss.
| .expect("Reward event should be rkyv deserializable"); | ||
|
|
||
| let total_block_rewards: Dusk = rewards.iter().map(|r| r.value).sum(); | ||
| let total_block_rewards_f64: f64 = from_dusk(total_block_rewards); |
There was a problem hiding this comment.
If you look at get_block_rewards in rusk/src/lib/node.rs, line 115, the block rewards include the fees amount. Reward value = new emissions + transaction fees. The fee amount is distributed by the reward % split (10/70/10/10 as described in the doc comment above get_block_rewards).
With the current implementation you inflate the supply as you include the fees/dusk_spent. You would need to subtract dusk_spent from total_block_rewards before adding it to total_supply so only the emission is counted as new supply.
| const PERIOD_BLOCKS: u64 = 12_614_400; | ||
|
|
||
| // Emission per complete period in Dusk | ||
| const P_1_EMISSION: f64 = 250_480_000.0; |
There was a problem hiding this comment.
If you look at /rusk/src/llib/node.rs, lines 143-180, you'll see that the emission numbers you use here are off. For the first period it's about ~9187 DUSK, second period ~4593 DUSK, etc.
For the supply burn this means you'll underestimate. Instead, I would maybe consider reusing the emission_amount function in Rusk, and applying a similar dusk function like is done in the test there to prevent any issues with floating point math.
| event.event.target == dusk_core::stake::STAKE_CONTRACT | ||
| && event.event.topic == "reward" | ||
| }) | ||
| .expect("Every block needs a reward event") |
There was a problem hiding this comment.
Not sure what to think of this. This should never happen, but in case it ever does it would hard crash the archive node.
2 participants
No description provided.